Skip to content

fix: Boundary condition error in VerifyStream #509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

seebees
Copy link
Contributor

@seebees seebees commented Jan 8, 2021

Fixes: #507

Tests exist that push a single byte at a time.
This will hit every boundary condition.
However this is not complete enough.
State can be reset as the code transitions
from one boundary condition to another.

The prime example of this problem
the VerifyStream.
It has 4 boundary in the _transform function.
The body header, the body, the auth tag, and signature.
By sending 1 byte
as the code transitions from the body header
to the body
the code handling the exiting
of the body header has no way to interact
with the body,
because the 1 byte of data
completely isolates these code branches.

In this case the code was buffering
the body header in frameBuffer
but when exiting this section
and passing control to
the actual content section,
it used the chunk to form the tail.

When passing a single byte,
this tail would always be empty,
because it would have been incorporated
into the frame header.

Adding tests to span
the boundary conditions
to verify both this fix
and that there are not other lurking.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

Fixes: aws#507

Tests exist that push a single byte at a time.
This will hit every boundary condition.
However this is not complete enough.
State can be reset as the code transitions
from one boundary condition to another.

The prime example of this problem
the `VerifyStream`.
It has 4 boundary in the `_transform` function.
The body header, the body, the auth tag, and signature.
By sending 1 byte
as the code transitions from the body header
to the body
the code handling the exiting
of the body header has no way to interact
with the body,
because the 1 byte of data
completely isolates these code branches.

In this case the code was buffering
the body header in `frameBuffer`
but when exiting this section
and passing control to
the actual content section,
it used the `chunk` to form the tail.

When passing a single byte,
this tail would _always_ be empty,
because it would have been incorporated
into the frame header.

Adding tests to span
the boundary conditions
to verify both this fix
and that there are not other lurking.
@seebees seebees requested a review from robin-aws January 8, 2021 20:08
Co-authored-by: Robin Salkeld <salkeldr@amazon.com>
@seebees seebees merged commit f177cc9 into aws:master Jan 28, 2021
@seebees seebees deleted the streaming-boundary-case branch January 28, 2021 20:51
robin-aws pushed a commit that referenced this pull request Apr 23, 2021
Fixes: #507

Tests exist that push a single byte at a time.
This will hit every boundary condition.
However this is not complete enough.
State can be reset as the code transitions
from one boundary condition to another.

The prime example of this problem
the `VerifyStream`.
It has 4 boundary in the `_transform` function.
The body header, the body, the auth tag, and signature.
By sending 1 byte
as the code transitions from the body header
to the body
the code handling the exiting
of the body header has no way to interact
with the body,
because the 1 byte of data
completely isolates these code branches.

In this case the code was buffering
the body header in `frameBuffer`
but when exiting this section
and passing control to
the actual content section,
it used the `chunk` to form the tail.

When passing a single byte,
this tail would _always_ be empty,
because it would have been incorporated
into the frame header.

Adding tests to span
the boundary conditions
to verify both this fix
and that there are not other lurking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KMS Stream Decryption Error
2 participants